filesys: a handle on an existing file is writable#259
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes HOSTFS semantics for opening existing files so that handles returned via OpenFromLock() and Open(MODE_OLDFILE) are writable (matching AmigaDOS behavior). This unblocks Workbench icon snapshotting on HOSTFS volumes, which previously failed due to a host-side read-only handle.
Changes:
- Route
ACTION_FH_FROM_LOCKandACTION_FINDINPUTthrough a newopen_existing()helper to prefer read/write opens (with a read-only fallback). - Add a focused regression test that exercises both open paths and verifies a real write succeeds and persists on disk.
- Refactor/DRY some filesystem packet-driving test setup via a new
FsTesterhelper.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Open(MODE_OLDFILE) and OpenFromLock() both opened the host file read-only, so any write through the handle failed with a host error that maps to ERROR_SEEK_ERROR. On AmigaOS both hand out a read/write handle, which Workbench's Snapshot relies on: icon.library locks the .info, OpenFromLock()s it, reads the header and writes it back through the same handle. Open read/write, falling back to read-only if the host refuses; the write then fails, as it does for a protected file on the Amiga. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Shorten open_existing's comment to the behaviour rather than the one bug that motivated it. Factor the packet-driving boilerplate the writable-handles and seek tests duplicated into an FsTester harness (mount, startup packet, send, name/buffer helpers, temp-dir cleanup on drop), and drop the redundant _like_iffparse suffix from the seek test's name. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
codewiz
force-pushed
the
fix/filesys-writable-handles
branch
from
July 23, 2026 06:53
093fb03 to
351bc95
Compare
open_existing fell back to a read-only handle on any error from the read/write open, which could mask an unrelated failure as success and discarded the original error. Fall back only for PermissionDenied and ReadOnlyFilesystem -- a read-only file or host mount, the cases we mean to open read-only -- and propagate anything else unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LinuxJedi
approved these changes
Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Workbench could not snapshot icons on a HOSTFS volume: the Snapshot ended
in a seek error.
The packet trace shows icon.library's path -- lock the .info, OpenFromLock
it, read the header, write it back through the same handle:
ACTION_FH_FROM_LOCKopened the host file withFile::open, i.e.read-only, so
write_allfailed with a host error thathost_errorfallsthrough to
ERROR_SEEK_ERROR.ACTION_FINDINPUThad the same bug:Open(MODE_OLDFILE)yields a read/write handle on the Amiga, andOpenFromLock's handle is writable regardless of the lock's access mode.
Both now go through
open_existing(), which opens read/write and fallsback to read-only if the host refuses -- a later write then fails, which
is what a protected file does on the Amiga too.
Covered by a new test driving both open paths through a real write and
checking the bytes on disk.
🤖 Generated with Claude Code